refactor(app): remove orphaned timeline header code#675
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request refactors the MessageTimeline component by removing orphaned logic related to session titles, headers, and exports, which helps resolve the file's size constraints as noted in the updated frontend architecture manifest. Feedback identifies that the imports for messageAgentColor and makeTimer are now also unused and should be removed to complete the cleanup.
Perf delta summaryComparator: pass
|
ac9bea9 to
db3cbf5
Compare
0bca5d3 to
aad77f6
Compare
db3cbf5 to
7a78f36
Compare
Goal: extract MessageTimeline turn-change ownership into a dedicated session-turn-changes module while preserving visible behavior and stack scope.\n\nBoundary: only touches the message timeline wiring, the new turn-change owner module, and focused tests. This does not change UI/product behavior, persistence, platform behavior, dependencies, release assets, or the downstream #675 scope.\n\nVerification: focused tests passed with bun test --preload ./happydom.ts src/pages/session/session-turn-changes.test.ts src/pages/session/session-message-comments.test.ts; bun run typecheck passed; targeted #600 timeline perf probe passed locally; git diff --check passed; GitHub required checks all passed including perf-probe-baseline.\n\nReview follow-up: Gemini blocked-copy separator threads were fixed and resolved. Fresh-eyes review found a locale separator P2, fixed in 8981175 so real locales own punctuation; the follow-up fresh-eyes review was clean with no P0-P3 findings.\n\nResidual risk: behavior is intended unchanged. Remaining risk is limited to moved undo/redo turn-change fetch and conflict-confirm retry wiring, covered by focused tests and required CI.
aad77f6 to
1469720
Compare
1469720 to
2c7ccab
Compare
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/pages/session/message-timeline.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
Topology Update (2026-05-16)
Final base:
devafter #674 was squash merged.Dependency status: true stack dependency. The deleted header/controller block and the below-500 LOC claim are only review-clean after the preceding message-timeline extractions.
Review order: final PR in the retained message-timeline stack after #674.
Latest head after restack and review follow-up:
2c7ccaba4.Summary
Removed the orphaned title/export/header controller block from
MessageTimeline. The deleted code had no JSX call site in the current timeline render path, and this stacked PR bringsmessage-timeline.tsxbelow the 500 LOC warning line.Why
This continues the #601 message-flow architecture stack after #674. Before this PR,
MessageTimelinestill carried unused title editing, parent-title lookup, session export, and menu state code even though the rendered timeline no longer called those handlers or components. Keeping that block made future agents read a false responsibility boundary.Canonical manifest:
.github/frontend-architecture-manifest.mdRelated Issue
Human Review Status
Fresh-eyes review clean after restack to
devand Gemini follow-up. No P0-P3 findings. The final2c7ccaba4push is a tree-identical amend of reviewed146972006to retrigger missing pull_request checks; no code changed after review.Review Focus
MessageTimelinerender behavior is unchanged: jump button,ScrollView, staging, comments, andSessionTurnwiring remain intact.devafter refactor(app): extract session turn changes #674 was squash merged.Risk Notes
Behavior is intended unchanged. Main risk is accidentally deleting a dormant feature entry point; the current file had no JSX path using these handlers or state. No dependency, platform, persistence, or migration change.
Architecture Boundary
Owner lane: #601 message flow.
Base/depends on: #667 -> #669 -> #670 -> #671 -> #672 -> #674. This PR is now restacked on
dev.Touched files:
packages/app/src/pages/session/message-timeline.tsxArchitecture effect:
message-timeline.tsxis below the 500 LOC warning line after this stack sliceReview Follow-up
tint/workingStatusblock and its imports. The thread was replied to and resolved.message-timeline.tsx, does not duplicate previous stack changes, and leaves export/rename entry points in the sidebar/layout path.How To Verify
Screenshots or Recordings
Not included. This PR removes unreachable controller code and has no intended visible UI or copy change, so Electron manual visual verification was not run.
Checklist
devafter the merged stack base, and my PR title and commit messages use Conventional Commits in English